FormatWriter: format multiline docstrings#1987
Conversation
olafurpg
left a comment
olafurpg
left a comment
There was a problem hiding this comment.
This is exciting! A few more test cases that I think would be nice to cover.
- code formatted with backticks (single line and multiline). Even if it’s not official scaladoc markup syntax I’ve seen it used several places and it renders as expected in some cases.
- quoted strings (single and double), whether they get line wrapped or not we should document the behavior in the tests.
- URLs followed by a white space and period. Just make sure not to remove the space.
- HTML markup, white space inside HTML is significant so we should consider leaving it unchanged. Otherwise we may want to consider writing a full blown XML pretty printer.
kitbellew
left a comment
kitbellew
left a comment
There was a problem hiding this comment.
- code formatted with backticks (single line and multiline). Even if it’s not official scaladoc markup syntax I’ve seen it used several places and it renders as expected in some cases.
- quoted strings (single and double), whether they get line wrapped or not we should document the behavior in the tests.
- URLs followed by a white space and period. Just make sure not to remove the space.
- HTML markup, white space inside HTML is significant so we should consider leaving it unchanged. Otherwise we may want to consider writing a full blown XML pretty printer.
will do.
Create a class and move the existing method there. We will subsequently add methods to format and wrap docstrings.
Added a test for single backquotes. In order to support triple backquotes, we need to modify the parser in scalameta; however, I am not sure how valid or useful that would be, as Intellij doesn't support them, and they don't seem to be supported in any scaladoc documentation (https://docs.scala-lang.org/overviews/scaladoc/for-library-authors.html#markup, http://keithpinson.github.io/Scaladoc-HOWTO/api/docSample/syntax.html)
Added both; there's no special handling for now, they are wrapped as any other space-separated words.
Added; no special handling, space separates the period into a separate word.
Added a test but I doubt this is what you had in mind. To support, we need to extend the scaladoc parser in scalameta; there's Ideally, if we are to support html, we'd have to parse I'd address this in a subsequent version, as that's highly non-trivial. |
There was a problem hiding this comment.
I agree XML parsing is non-trivial and it's not clear whether it's worth the investment. Just having tests to document the current behavior is a good start.
It might be worth mentioning in the docs that HTML tags are formatted just like normal text, which may produce unexpected output for tables and other large HTML elements.
kitbellew
left a comment
kitbellew
left a comment
There was a problem hiding this comment.
It might be worth mentioning in the docs that HTML tags are formatted just like normal text, which may produce unexpected output for tables and other large HTML elements.
i have some warning to that effect, please see below; should it be revised?
| > This functionality is generally limited to | ||
| > [standard scaladoc elements](https://docs.scala-lang.org/overviews/scaladoc/for-library-authors.html) | ||
| > and might lead to undesirable results in corner cases; | ||
| > for instance, the scaladoc parser doesn't have proper support of embedded HTML. |
There was a problem hiding this comment.
@olafurpg would you like to amend this statement to say it will be treated as normal text?
There was a problem hiding this comment.
perfect, i will merge.
separately, i will add table support to scalameta and then propagate it here. does this sound reasonable: https://www.scala-lang.org/blog/2018/10/04/scaladoc-tables.html
|
Hi even after taking latest release i am still seeing issue #1387? Do I need to something more? |
please submit an issue providing all of the information, including a reproducible example. |
Thanks for replying. Sure will raise one soon. |
3 participants
Now that scalameta in 4.3.13 supports parsing of docstrings, let's use that and format.
Fixes #1387.
Fixes #1887.